debug log changes #39
Conversation
NeosModLoader/NeosMod.cs
Outdated
| @@ -6,6 +6,8 @@ namespace NeosModLoader | |||
| // contains members that only the modloader or the mod itself are intended to access | |||
| public abstract class NeosMod : NeosModBase | |||
| { | |||
| public static bool IsDebugEnabled() => Logger.IsDebugEnabled(); | |||
| public static void DebugFunc(Func<string> messageProducer) => Logger.DebugFuncExternal(messageProducer); | |||
There was a problem hiding this comment.
I think that the DebugFunc could be generalized a bit more, allowing any return types instead of just strings? And perhaps DebugFuncExternal could skip the debugging if null is returned, allowing that to be used as a shorthand for just running debugging functions without needing for it to be logged?
There was a problem hiding this comment.
I could see allowing object parameters, but I don't think dropping nulls is what people would want from a Debug log API. For example, if someone is trying to do a quick and dirty DebugFunc(() => someFrooxObject) and that object is null, I think they'd much rather have the null logged than silently dropped.
NeosModLoader/Logger.cs
Outdated
| internal static void DebugFuncInternal(Func<string> messageProducer) | ||
| { | ||
| if (IsDebugEnabled()) | ||
| { | ||
| LogInternal(LogType.DEBUG, messageProducer()); | ||
| } | ||
| } | ||
|
|
||
| internal static void DebugFuncExternal(Func<string> messageProducer) | ||
| { | ||
| if (IsDebugEnabled()) | ||
| { | ||
| LogInternal(LogType.DEBUG, messageProducer(), SourceFromStackTrace()); | ||
| } | ||
| } |
There was a problem hiding this comment.
I very much dislike the code duplication of this, but then I guess it's kind of a necessary evil, as they could deviate more in the future, and passing bool arguments isn't good either.
There was a problem hiding this comment.
Yeah, the methods are a bit ugly, I agree, but I can't think of a better way of dealing with it. The good news is this crusty API is internal so we can always refactor it later if we have any ideas.
| @@ -23,7 +23,7 @@ internal static void DebugFuncInternal(Func<string> messageProducer) | |||
| } | |||
| } | |||
|
|
|||
| internal static void DebugFuncExternal(Func<string> messageProducer) | |||
| internal static void DebugFuncExternal(Func<object> messageProducer) | |||
There was a problem hiding this comment.
Should probably check if the output of messageProducer() is null and in that case also not log?
There was a problem hiding this comment.
the output of messageProducer() is already being null-checked, and it logs the string "null"
There was a problem hiding this comment.
So then DebugFuncExternal's output will always be logged. I guess it's a sensible option, just thought it might be neat to allow it to be used with potentially no output.
Originally part of #38, but I've separated the two